Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sparklines #622

Merged
merged 4 commits into from
Nov 12, 2015
Merged

Sparklines #622

merged 4 commits into from
Nov 12, 2015

Conversation

paulbellamy
Copy link
Contributor

Initial work on sparklines. For this, they are just an in-memory set of [timestamp, float] tuples. Merge semantics are easy, as it is just set-union with timestamp for uniqueness. Timestamps expire as reports expire.

The graphs are a bit sub-optimal, but probably good enough to be useful, so will keep relatively basic for first release.

Todo:

  • Clean up code
  • Add tests
  • Add other sparklines (will need some generalizing of the code)
  • UI work (in particular, render "latest value" inline after sparkline)
  • Squash commits

Potential other sparklines to add:

  • container memory usage
  • cpu usage (do we report this)
  • Threads

  • TCP Connections

  • stuff from docker?

At the moment, because we get new data every 15 seconds (and the sparklines are 15 seconds long), every 15 seconds the sparklines entirely redraw. It would be nice to smooth that out.

@paulbellamy paulbellamy force-pushed the timeseries branch 2 times, most recently from ed484ff to 93aec34 Compare November 5, 2015 12:13
Load string `json:"load"`
Load1 string `json:"load1"`
Load5 string `json:"load5"`
Load15 string `json:"load15"`

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

Another thought: we might want to come up with a way to deal with units - memory usage will be in bytes, but we might want to display it in bytes. Similarly, CPU in %, with min and max pegged at 0 and 100, or latency in ms, or QPS in... QPS.

@@ -220,6 +222,48 @@ func (c *container) ports(localAddrs []net.IP) report.StringSet {
return report.MakeStringSet(ports...)
}

func (c *container) cpuPercentMetric() report.Metric {
result := report.MakeMetric()
if len(c.pendingStats) < 2 {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

scale float64
}{
{docker.MemoryUsage, "Memory Usage", "%0.2f MB", 1024 * 1024.},
{docker.CPUTotalUsage, "CPU Usage", "%0.2f%%", 1.},

This comment was marked as abuse.

@tomwilkie tomwilkie added this to the 0.10.0 milestone Nov 5, 2015
@paulbellamy paulbellamy force-pushed the timeseries branch 3 times, most recently from 0450a54 to 48f6b03 Compare November 6, 2015 16:26
@paulbellamy paulbellamy changed the title [WIP] Sparklines Sparklines Nov 6, 2015
@paulbellamy
Copy link
Contributor Author

First two commits are pulled out into: #631

@tomwilkie @davkal , PTAL, thanks.

{ row.value_type === 'sparkline' && <div className="node-details-table-row-value-sparkline"><Sparkline data={row.metric.samples} min={0} max={row.metric.max} first={row.metric.first} last={row.metric.last} interpolate="none" />{row.value_major}</div> }
{ row.value_type === 'sparkline' && <div className="node-details-table-row-value-unit">{row.value_minor}</div> }
{ row.value_type !== 'numeric' && row.value_type !== 'sparkline' && <div className="node-details-table-row-value-major truncate" title={row.value_major}>{row.value_major}</div> }
{ row.value_type !== 'numeric' && row.value_type !== 'sparkline' && row.value_minor && <div className="node-details-table-row-value-minor truncate" title={row.value_minor}>{row.value_minor}</div> }

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@davkal
Copy link
Contributor

davkal commented Nov 9, 2015

Metric data are sent even for non-metric fields:

{
            "key": "Operating system",
            "value_major": "darwin",
            "metric": {
              "samples": null,
              "min": 0,
              "max": 0,
              "first": "0001-01-01T00:00:00Z",
              "last": "0001-01-01T00:00:00Z"
            }
          },

This roughly doubles the response size of a node details request compared to master.

@davkal
Copy link
Contributor

davkal commented Nov 9, 2015

I'd change the width to 48:
screen shot 2015-11-09 at 12 42 34

Otherwise they are too dominant.

@davkal
Copy link
Contributor

davkal commented Nov 9, 2015

I'd also add title attribute so that a tooltip shows the length of the timeseries.

@paulbellamy
Copy link
Contributor Author

@davkal by "length" do you mean number of samples, timespan, or both?

@davkal
Copy link
Contributor

davkal commented Nov 9, 2015

@paulbellamy whatever we can say for sure. At the minimum "Last 15 secs", even better "Last 15 sec, 1 sample/sec, min: 0.6, max: 1.8, avg: 1.2".

@paulbellamy paulbellamy force-pushed the timeseries branch 2 times, most recently from 824d3a4 to 7dea1f6 Compare November 9, 2015 14:53
@paulbellamy paulbellamy force-pushed the timeseries branch 6 times, most recently from de528b0 to ecd223a Compare November 10, 2015 13:49
@@ -43,7 +43,7 @@ $(PROBE_EXE): probe/*.go probe/docker/*.go probe/kubernetes/*.go probe/endpoint/

ifeq ($(BUILD_IN_CONTAINER),true)
$(APP_EXE) $(PROBE_EXE) $(RUNSVINIT): $(SCOPE_BACKEND_BUILD_UPTODATE)
$(SUDO) docker run $(RM) -v $(shell pwd):/go/src/github.com/weaveworks/scope -e GOARCH -e GOOS \
$(SUDO) docker run $(RM) -ti -v $(shell pwd):/go/src/github.com/weaveworks/scope -e GOARCH -e GOOS \

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@davkal
Copy link
Contributor

davkal commented Nov 10, 2015

Tooltip looks great! I'd still vote for smaller width, e.g. 48px. Otherwise LGTM! Well done, jsx ❤️ Paul

if m.Last.IsZero() || t.After(m.Last) {
m.Last = t
}
return m

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

When you're ready, I think this need to be squashed and split into 3 PRs - the plumbing (changes to report and render/ etc), the exposing of metrics in Docker and Host probe packages, and the UI work.

@paulbellamy paulbellamy force-pushed the timeseries branch 2 times, most recently from 00ce461 to 3b3a45c Compare November 11, 2015 17:00
paulbellamy and others added 4 commits November 11, 2015 17:16
- base load graph x-axis on data, not a hardcoded window
- format memory dynamically to scale
- add some tests for the immutability of metrics
- use ps.List for Sample storage, so it is immutable. Have to implement custom marshalling
- adding tests for report.Metrics
- check the ordering of the json samples
- Make the nil value for Metrics valid.
- Sort samples from oldest to newest on the wire.
- basic sparklines rendering for load
- Graphs are normalized so they all render on the y-axis.
- Time-axis is fixed to 15-seconds, so that data fills in correctly when data is insufficient
- Move load scalar behind sparkline
- add title to sparklines, showing timespan, samples, etc
@tomwilkie
Copy link
Contributor

Assuming this is green, LGTM.

paulbellamy added a commit that referenced this pull request Nov 12, 2015
@paulbellamy paulbellamy merged commit 5c606c4 into master Nov 12, 2015
@paulbellamy paulbellamy deleted the timeseries branch November 12, 2015 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants